-
Notifications
You must be signed in to change notification settings - Fork 73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/ocrvs 7978/qr reader #8196
base: develop
Are you sure you want to change the base?
Conversation
/uikit |
/uikit |
Storybook deployed: https://feat-ocrvs-7978-qr-reader.opencrvs.pages.dev |
display: flex; | ||
align-items: center; | ||
width: ${width || '100%'}; | ||
|
||
:before, :after { | ||
flex: 1; | ||
content: ''; | ||
padding: 0 1px 1px; | ||
background-color: ${color || theme.colors.grey200}; | ||
margin: 16px; | ||
} | ||
|
||
:before { | ||
margin-left: 0; | ||
} | ||
|
||
:after { | ||
margin-right: 0; | ||
} | ||
` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically what does this do if Divider has children? Could this be documented in Storybook 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const renderReader = (reader: ReaderType) => { | ||
const { type, ...readerProps } = reader | ||
if (type === 'qr') { | ||
return ( | ||
<QRReader | ||
key={type} | ||
{...readerProps} | ||
onScan={onScan} | ||
onError={onError} | ||
/> | ||
) | ||
} else return null | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a "ReaderGenerator" (like FormFieldGenerator), can be lifted to a separate component so it doesn't get re-created on every render of IDReader
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, I am on it 👍🏼
<Icon name={props.iconName} size="large" /> | ||
</IconContainer> | ||
<Text variant="reg14" element="p"> | ||
{props.label} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{props.label} | |
{children} |
const StyledButton = styled(SecondaryButton)` | ||
width: 100%; | ||
` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Button
component instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thanks. will do
…ders, i.e. QR, eSignet
@euanmillar @naftis now link button will use both the params defined in form config and those came after the redirection when calling the callback URL For instance, for a callback defined below
After the redirection url sends back a code when to opencrvs form When the form is rendered with the link button, the link button looks in the param
Please let me know if there is any improvement needed |
function checkParamsPresentInURL() { | ||
for (const [key, value] of Object.entries(params)) { | ||
if (urlParams.get(key) !== value) { | ||
return false | ||
} | ||
} | ||
return true | ||
} | ||
if (checkParamsPresentInURL() && !hasCallbackRequestBeenMade) { | ||
call({ | ||
// forward params which are received after redirection to the callback request | ||
params: Object.fromEntries(urlParams) | ||
}) | ||
setCallbackRequestBeenMade(true) | ||
} | ||
}, [call, params, form, trigger, hasCallbackRequestBeenMade]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tahmidrahman-dsi how / where is this call/params functionality documented? It's a good method, but can be confusing for someone wanting to use the component.
/** | ||
* If the redirection url has the exact same param keys | ||
* with exact same values sepecified in the below `params` | ||
* field, only then the callback will be triggered | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const Scanner = (props: ScannerProps) => { | ||
const scanner = useRef<QRScanner>() | ||
const videoElement = useRef<HTMLVideoElement>(null) | ||
const [qrOn, setQrOn] = useState(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I read this correctly, qrOn can never be false. When the camera cannot be used, we need to consider the error cases a bit. Let's create a new ticket on that and solve it later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thanks @naftis that is true and because of the fact that qrOn
is never false,
if (!qrOn) alert('Could not scan') |
this line becomes irrelevant. We might need to determine how to handle that case as the current way is showing an alert which was an interim change and not sure if it should stay like this
Issue created: #8330
Reason for this feature
To support countries which implemented identity systems to obtain identifiers for its residents. A common use case is that there is a QR code associated with the resident's identity card provided by the identity system. The QR code holds the person information and can be used to pre-fill event form. We need to support this use case.
What this feature does
This feature introduces QR code scanning ability to the application via a UI component called
<IDReader />
which can be used in form with typeID_READER
.Bundle size effects
Before (develop):
After (feature branch merged with develop)
Merging this ticket closes #7978, #7979